Осипов Даниил ИТМО ФИТИП HW5 #188
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
src/main/java/ru/vk/itmo/test/osipovdaniil/MergeHandleResult.java
Outdated
Show resolved
Hide resolved
| instance.start().get(1, TimeUnit.SECONDS); | ||
| } catch (InterruptedException e) { | ||
| Thread.currentThread().interrupt(); | ||
| throw new RuntimeException(e); |
There was a problem hiding this comment.
Define and throw a dedicated exception instead of using a generic one.
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.ExecutorService; |
There was a problem hiding this comment.
Avoid unused imports such as 'java.util.concurrent.ExecutorService'
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.ExecutorService; | ||
| import java.util.concurrent.TimeUnit; |
There was a problem hiding this comment.
Avoid unused imports such as 'java.util.concurrent.TimeUnit'
| public boolean add(int index, HandleResult handleResult) { | ||
| handleResults[index] = handleResult; | ||
| int valid = validateResultStatus(handleResult.status()) ? countValid.getAndIncrement() : countValid.get(); | ||
| if (valid >= ack) { |
There was a problem hiding this comment.
Получается, если у тебя ack = 3, а from = 5, то ты 3 раза сделаешь sendResult. Лучше проверяй не на >=, а на =
| : HttpRequest.BodyPublishers.ofByteArray(request.getBody()) | ||
| ) | ||
| .header(HEADER_REMOTE, "true") | ||
| .timeout(Duration.ofMillis(500)) |
There was a problem hiding this comment.
Все числа в константы. В твоём случае лучше вообще Duration.ofMillis(500), чтобы не аллоцировать каждый раз
| } | ||
|
|
||
| for (int i = count; i < config.clusterUrls().size(); i++) { |
There was a problem hiding this comment.
А почему не посчитать хеш от всех clusterUrls, отсортировать и взять первые count?
There was a problem hiding this comment.
А зачем, у нас будет count * log(count), а так count + count * (cluster.size() - count). Тут ситуативно + у нас дефолтное значение count как раз cluster.size(), то есть я считаю что у нас достаточно случаев когда эта реализация работает лучше
| } | ||
|
|
||
| void shutdownAndAwaitTermination(ExecutorService pool) { | ||
| private void shutdownAndAwaitTermination(ExecutorService pool) { | ||
| try { | ||
| if (!pool.awaitTermination(60, TimeUnit.MILLISECONDS)) { | ||
| pool.shutdownNow(); | ||
| if (!pool.awaitTermination(60, TimeUnit.MILLISECONDS)) { |
There was a problem hiding this comment.
тут тоже все числа в константы
| @@ -0,0 +1,49 @@ | |||
| # Отчёт по 5 стейджу. | |||
|
|
|||
| К сожалению, так как 4 стейдж я не делал, сравнений не будет, | |||
There was a problem hiding this comment.
В 3 стейдже ты шардировал свою систему.
- Попробуй в том стейдже выключить один шард, посмотри сколько будет неудачных (статус не 200 и не 300) запросов
- В этом стейдже попробуй выключить один шард, посмотри сколько будет неудачных запросов
There was a problem hiding this comment.
Добавил в конец отчёта
| @@ -29,7 +29,7 @@ public MergeHandleResult(HttpSession session, int size, int ack) { | |||
| public boolean add(int index, HandleResult handleResult) { | |||
| handleResults[index] = handleResult; | |||
| int valid = validateResultStatus(handleResult.status()) ? countValid.getAndIncrement() : countValid.get(); | |||
There was a problem hiding this comment.
Единственное, тебе нужен incrementAndGet иначе тебе будет нужно ack + 1 ответов и например, если у тебя ack = 2, from = 3, то придётся ждать пока доработает последний
No description provided.